Skip to content

More detail on append() aborting and other semantics #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2025
Merged

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Apr 21, 2025

See discussions in #92 (comment) onward.

@lozy219 PTAL! Also cc @clarkduvall.

Copy link
Contributor

@lozy219 lozy219 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The change looks good to me.

@clarkduvall
Copy link

I think it may be fine to remove for now, but abort() is definitely something we're going to want to support eventually. See my comment on #92 (comment), abort is important for cancelling expensive append() operations.

@domenic domenic changed the title Remove append() aborting and add more detail on append More detail on append() aborting and other semantics Apr 22, 2025
@domenic
Copy link
Collaborator Author

domenic commented Apr 22, 2025

Needs re-review now that I've redone this to allow aborting.

@clarkduvall
Copy link

new wording lgtm, thanks!

@domenic domenic merged commit 2411e63 into main Apr 24, 2025
1 check passed
@domenic domenic deleted the append-no-abort branch April 24, 2025 03:45
aarongable pushed a commit to chromium/chromium that referenced this pull request May 9, 2025
This CL aligns the implementation with
webmachinelearning/prompt-api#95 and
webmachinelearning/prompt-api#104

Bug: 409355678
Change-Id: I0c119242b5f3fb2e01feada1852a6e7b57a9d3ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6468355
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Mingyu Lei <[email protected]>
Reviewed-by: Clark DuVall <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1457987}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants